Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add itest for MinRelayFee check in tapd #892

Merged
merged 3 commits into from
Nov 5, 2024
Merged

Conversation

gijswijs
Copy link
Contributor

@gijswijs gijswijs commented Oct 30, 2024

This commit adds an integration test for the MinRelayFee check. The test ensures that transactions with fees below the minimum relay fee are rejected.

This PR depends on lightninglabs/taproot-assets#1163 which in turn depends on lightninglabs/lndclient#200

It uses pseudoversions in go.mod for both taproot-assets and lndclient that should be updated before merging.

@guggero guggero force-pushed the update-to-lnd-18-4 branch from 9c10445 to a4d76ed Compare October 31, 2024 16:00
@guggero guggero force-pushed the update-to-lnd-18-4 branch from 6b38cf0 to 3a8f1f6 Compare October 31, 2024 19:35
@gijswijs gijswijs force-pushed the validate-fee branch 3 times, most recently from 2506456 to de86073 Compare November 4, 2024 12:59
@gijswijs gijswijs marked this pull request as ready for review November 4, 2024 13:57
@gijswijs
Copy link
Contributor Author

gijswijs commented Nov 4, 2024

I've removed the default value of 1 for sat_per_vbyte that's being passed to litd rpc endpoint fundchannel. litd handles this now perfectly on its own, and adding logic here only adds to the confusion.

@gijswijs gijswijs requested review from jharveyb and guggero November 4, 2024 13:59
@dstadulis dstadulis added this to the tapd v0.4.2 milestone Nov 4, 2024
Copy link

@jharveyb jharveyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏽

One related comment about how we handle this feerate arg, but it can be addressed in a separate PR.

itest/litd_custom_channels_test.go Show resolved Hide resolved
This commit removes the default value of 1 for `sat_per_vbyte` that's
being passed to `litd` rpc endpoint `fundchannel`. `litd` handles this
now perfectly on its own, and adding logic here only adds to the
confusion.
This commit adds an integration test for the MinRelayFee check. The test
ensures that transactions with fees below the minimum relay fee are
rejected.
},
)

errFeeRateTooLow := fmt.Sprintf("fee rate %s too low, "+
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually shows a problem with the sat/vByte fee rate... Because 1 sat/vByte is equal to 250 sat/kw but the absolute minimum relay fee there can ever be is defined as 253 sat/kw, this would mean that you could only fund channels with at least 2 sat/vBytes. Which is kind of unfortunate.
But I think we should fix this in tapd and unrelated to this PR. Just letting you know that this might change again in the future.

@guggero guggero merged commit f308629 into update-to-lnd-18-4 Nov 5, 2024
13 checks passed
@guggero guggero deleted the validate-fee branch November 5, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants